[SYCL] Drop kernel name type backward compatibility #20713
Conversation
sergey-semenov
requested review from
a team,
aelovikov-intel and
cperkinsintel
as code owners
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
sergey-semenov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| #ifndef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| /// \return a string containing name of SYCL kernel. | ||
| detail::ABINeutralKernelNameStrT getKernelName(); | ||
| detail::string_view getKernelName(); |
There was a problem hiding this comment.
Why aren't you dropping this entire #ifndef?
There was a problem hiding this comment.
There's quite a lot of legacy code that would need to be removed alongside this, which I'd prefer to be done separately.
There was a problem hiding this comment.
I'll create a separate PR for this, to be landed before this one.
| auto [Iter, Inserted] = | ||
| m_DeviceKernelInfoMap.try_emplace(KernelNameStrT{Info.Name.data()}, Info); | ||
| auto [Iter, Inserted] = m_DeviceKernelInfoMap.try_emplace( | ||
| std::string_view{Info.Name.data()}, Info); |
There was a problem hiding this comment.
I had tried to add detail::string_view::operator std::string_view but it resulted in ambiguities with some comparison operators (I think). Now that we can remove old ones, maybe a better fix would be to change detail::string_view to have this conversion and rely on it happening implicitly for comparisons.
There was a problem hiding this comment.
Right, I recall running into something along those lines. On this line in particular though, there's no conversion to be made at all now and I just didn't catch that at first.
| #include <sycl/detail/kernel_name_str_t.hpp> | ||
| #include <sycl/kernel_bundle.hpp> | ||
|
|
||
| #include <cstring> |
There was a problem hiding this comment.
I'm somewhat surprised we need this.
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
2 participants
No description provided.